Add unique-names anchor to fragments docs - #13336
Conversation
|
@atharv-sys32: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
|
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a “Unique fragment names” section to the fragments documentation, including the duplicate-name warning and naming guidance. Updates the changeset with a patch bump for ChangesFragment names documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/data/fragments.mdx`:
- Line 46: Update the duplicate-fragment explanation in the fragment
documentation to state that graphql-tag warns whenever the same fragment name is
reused, without qualifying it by different field selections. Preserve the
existing runtime-warning context and uniqueness requirement.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 311dfd1c-1396-439e-9967-dc234485b79a
📒 Files selected for processing (2)
.changeset/quiet-pans-doubt.mddocs/source/data/fragments.mdx
|
|
||
| ## Unique fragment names {#unique-names} | ||
|
|
||
| Fragment names must be unique across your entire application. If you accidentally define two fragments with the same name but different field selections, `graphql-tag` warns you about the conflict at runtime to prevent subtle bugs: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '40,55p' docs/source/data/fragments.mdxRepository: apollographql/apollo-client
Length of output: 1025
🏁 Script executed:
rg -n "unique across your entire application|different field selections|graphql-tag warns" -S docs/source/data/fragments.mdx . || trueRepository: apollographql/apollo-client
Length of output: 699
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('docs/source/data/fragments.mdx')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 40 <= i <= 55:
print(f"{i}: {line}")
PYRepository: apollographql/apollo-client
Length of output: 1089
🌐 Web query:
graphql-tag warns same fragment name different source identical fragment definitions warning implementation fragmentSourceMap
💡 Result:
The "Warning: fragment with name X already exists" message emitted by the graphql-tag library occurs because the package enforces global uniqueness for fragment names across your application [1][2][3]. Implementation Details: The library maintains an internal state using a structure often referred to as a fragmentSourceMap (or conceptually similar tracking mechanisms in its codebase) [2]. When you define a GraphQL document using the gql template literal, the library processes the document and inspects all fragment definitions [2]. For each fragment, it tracks the mapping between the fragment name and its source code [2]. If it encounters a fragment name that has already been registered with a different source definition, it triggers a console warning [2]. Even if the fragments are logically identical, the library's internal check may treat them as collisions if their source locations or registration contexts differ, leading to the warning [2][4]. How to Disable: If you are confident that your fragment names are intentionally reused or that the warnings are false positives in your architecture, you can disable these warnings by calling the disableFragmentWarnings function provided by the graphql-tag package [1][5][3]: import { disableFragmentWarnings } from 'graphql-tag'; disableFragmentWarnings; Note that if you are using a modern Apollo Client setup, you might occasionally need to import this from @apollo/client if it is bundled as a dependency, though importing directly from graphql-tag is the standard approach [1][6]. This warning is a legacy enforcement mechanism from earlier Apollo development practices and does not typically affect the runtime functionality of your GraphQL queries [4].
Citations:
- 1: https://github.com/apollostack/graphql-tag
- 2: https://github.com/apollographql/graphql-tag/blob/main/src/index.ts
- 3: https://github.com/apollographql/graphql-tag/blob/main/README.md
- 4: Fragment already exists warning #940
- 5: https://www.npmjs.com/package/graphql-tag
- 6: Using fragments in combination with
PreloadQueryleads tounique-nameswarnings apollo-client-integrations#328
Describe duplicate fragment names without the field-selection qualifier.
graphql-tag warns whenever the same fragment name is reused, so the “but different field selections” part is misleading and can imply identical duplicates are allowed.
🧰 Tools
🪛 GitHub Check: AI Style Review
[notice] 46-46: docs/source/data/fragments.mdx#L46
Framing Apollo Products: The phrase 'prevent subtle bugs' can be framed more positively as helping the user avoid them.
Voice: Use an authoritative and encouraging tone to prescribe the best practice.
Word and Symbol Usage: Avoid semicolons; use a period to separate independent clauses for better clarity.
| Fragment names must be unique across your entire application. If you accidentally define two fragments with the same name but different field selections, `graphql-tag` warns you about the conflict at runtime to prevent subtle bugs: | |
| Fragment names must be unique across your entire application. If you define two fragments with the same name but different field selections, `graphql-tag` warns you about the conflict at runtime to help you avoid bugs: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/source/data/fragments.mdx` at line 46, Update the duplicate-fragment
explanation in the fragment documentation to state that graphql-tag warns
whenever the same fragment name is reused, without qualifying it by different
field selections. Preserve the existing runtime-warning context and uniqueness
requirement.
| ## Unique fragment names {#unique-names} | ||
|
|
||
| Fragment names must be unique across your entire application. If you accidentally define two fragments with the same name but different field selections, `graphql-tag` warns you about the conflict at runtime to prevent subtle bugs: | ||
| Fragment names must be unique across your entire application. If you accidentally reuse a fragment name, `graphql-tag` warns you about the conflict at runtime to prevent subtle bugs: |
There was a problem hiding this comment.
| Fragment names must be unique across your entire application. If you accidentally reuse a fragment name, `graphql-tag` warns you about the conflict at runtime to prevent subtle bugs: | |
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` component will warn about the conflict at runtime to prevent subtle bugs: |
accidentally(as called out elsewhere) achieves the wrong thing. The callers I looked at were intentionally using the same term -- they just happened to use it to mean different things which this tool doesn't like.- Users (e.g., me) have no idea what
graphql-tagis. I didn't write<graphql-tag>foo...</graphql-tag>or<graphql>foo...</graphql>. We just used apollo. We land here because we used apollo and it spat out an error message.graphql-tagis an implementation detail that's bleeding outward.
…s resolve The graphql-tag library warns when duplicate fragment names are found and links to http://dev.apollodata.com/core/fragments.html#unique-names. This URL no longer resolves. Add a dedicated section with the {#unique-names} custom anchor ID to the Apollo Client fragments docs page so the link serves useful content instead of a 404. Fixes #13292
Remove 'accidentally' - the warning is about intentional reuse, and 'graphql-tag' is an implementation detail that users don't need to know about.
ccb1177 to
27a6ae5
Compare
jerelmiller
left a comment
jerelmiller
left a comment
There was a problem hiding this comment.
A couple small changes. Thanks for helping move this forward!
There was a problem hiding this comment.
Since this is a docs-only change, no need for a changeset. Appreciate it though!
There was a problem hiding this comment.
Removed the changeset file as requested.
|
|
||
| ## Unique fragment names {#unique-names} | ||
|
|
||
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` component will warn about the conflict at runtime to prevent subtle bugs: |
There was a problem hiding this comment.
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` component will warn about the conflict at runtime to prevent subtle bugs: | |
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` library will warn about the conflict at runtime to prevent subtle bugs: |
There was a problem hiding this comment.
Let's also use some of the AI suggestion. We tend to use present tense rather than future tense as well:
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` component will warn about the conflict at runtime to prevent subtle bugs: | |
| Fragment names must be unique across your entire application. If you reuse a fragment name, the `graphql-tag` library warns about the conflict at runtime to help prevent bugs: |
There was a problem hiding this comment.
Fixed - now uses "library warns" with present tense and "help prevent bugs" as you suggested.
| this in the docs: http://dev.apollodata.com/core/fragments.html#unique-names | ||
| ``` | ||
|
|
||
| Always use descriptive, component-scoped fragment names (like `ItemFragment` or `UserProfileFragment`) to avoid name collisions. |
There was a problem hiding this comment.
While I agree that this is a really good practice to do, I wouldn't make it as strong as "always". Let's soften that a bit to mention it as a best practice.
| Always use descriptive, component-scoped fragment names (like `ItemFragment` or `UserProfileFragment`) to avoid name collisions. | |
| As a best practice, use descriptive, component-scoped fragment names (like `ItemFragment` or `UserProfileFragment`) to avoid name collisions. |
There was a problem hiding this comment.
Fixed - changed to "As a best practice, use" to soften it.
- Use 'library' instead of 'component' for graphql-tag - Use present tense 'warns' instead of future tense - Use 'help prevent bugs' instead of 'prevent subtle bugs' - Soften 'Always use' to 'As a best practice, use' - Remove unnecessary changeset for docs-only change
|
Thanks @jerelmiller All three changes addressed:
|
|
Hey @jsoref! and @jerelmiller I had to close this PR temporarily due to a fork issue, but the changes are still available. I've created a fresh PR with the same fixes at #13365, marking it as draft for now so the more critical bug fix (#13364) can get attention first. All your previous feedback has been incorporated. Appreciate the review! |
4 participants
The
graphql-taglibrary warns when duplicate fragment names are found and links tohttp://dev.apollodata.com/core/fragments.html#unique-names. This URL no longer resolves.This PR adds a dedicated section with the
{#unique-names}custom anchor ID to the Apollo Client fragments docs page so that the old link serves useful content instead of a 404.Fixes #13292
Summary by CodeRabbit